Skip to content

Conversation

@ilyagr
Copy link
Contributor

@ilyagr ilyagr commented Jun 5, 2025

These are some docs that could have been helpful as I am adapting jj to #1935. Please feel free to edit them further.

I guess a part of this PR is a question: did I figure out correctly how I'm supposed to use these?

See also #1935 (comment). I wonder if some of these classes and methods should be renamed. (Perhaps it's possible to make the old name a deprecated type alias?)

@ilyagr
Copy link
Contributor Author

ilyagr commented Jun 5, 2025

I also noticed that there exists git_mailmap::Signature, but I'm guessing I should not be using that.

Then there's impl From<SignatureRef<'_>> for Signature {, which I perhaps should use instead of SignatureRef::to_owned() (perhaps that should be called parse?). I thought this was a TryFrom (Update: this is what the changelog for gix-actor said), but I guess that changed (or my IDE failed to find the TryFrom).

I'm also not sure whether the fact that the From implementation is lossy should be documented or not.

ilyagr added a commit to ilyagr/jj that referenced this pull request Jun 6, 2025
Adapt to changes in `gix-actor::Signature` and `gix-actor::SignatureRef`. The latter now contains an unparsed
string time field, while the former still contains a parsed time.
So, the conversions between `gix-actor::SignatureRef` and either `gix-actor::Signature` or jj's `Signature` types can now fail.

Cc: GitoxideLabs/gitoxide#1935, GitoxideLabs/gitoxide#2038
ilyagr added a commit to ilyagr/jj that referenced this pull request Jun 6, 2025
Adapt to changes in `gix-actor::Signature` and
`gix-actor::SignatureRef`. The latter now contains an unparsed string
time field, while the former still contains a parsed time.  So, the
conversions between `gix-actor::SignatureRef` and either
`gix-actor::Signature` or jj's `Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
ilyagr added a commit to ilyagr/jj that referenced this pull request Jun 6, 2025
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. Adapt to a change in signature of
   `gix::reference::iter::Platform::prefixed` that seems to confuse Rust
compiler.

2. Adapt to `git_object::Tree::EntryMode` API change; `entry.mode()` now
   has a `value()` method.

3. Most significantly, adapt to the changes to
   `gix::actor::SignatureRef`.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
ilyagr added a commit to ilyagr/jj that referenced this pull request Jun 6, 2025
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. Adapt to a change in signature of
   `gix::reference::iter::Platform::prefixed` that seems to confuse Rust
compiler.

2. Adapt to `git_object::Tree::EntryMode` API change; `entry.mode()` now
   has a `value()` method.

3. Most significantly, adapt to the changes to
   `gix::actor::SignatureRef`.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
ilyagr added a commit to jj-vcs/jj that referenced this pull request Jun 6, 2025
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. Adapt to a change in signature of
   `gix::reference::iter::Platform::prefixed` that seems to confuse Rust
compiler.

2. Adapt to `git_object::Tree::EntryMode` API change; `entry.mode()` now
   has a `value()` method.

3. Most significantly, adapt to the changes to
   `gix::actor::SignatureRef`.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
ilyagr added a commit to jj-vcs/jj that referenced this pull request Jun 6, 2025
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. Adapt to a change in signature of
   `gix::reference::iter::Platform::prefixed` that seems to confuse Rust
compiler.

2. Adapt to `git_object::Tree::EntryMode` API change; `entry.mode()` now
   has a `value()` method.

3. Most significantly, adapt to the changes to
   `gix::actor::SignatureRef`.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
ilyagr added a commit to jj-vcs/jj that referenced this pull request Jun 6, 2025
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. The signature of `gix::reference::iter::Platform::prefixed` changed
   in a way that seems to confuse Rust compiler (and does confuse me).

2. `git_object::Tree::EntryMode` API changed; `entry.mode()` now has a
   `value()` method.

3. Most significantly, the meaning and API of `gix::actor::SignatureRef`
   changed.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for improving the documentation!

I did edit it as suggested, mainly to be more specific where possible.

Then there's impl From<SignatureRef<'_>> for Signature {, which I perhaps should use instead of SignatureRef::to_owned() (perhaps that should be called parse?). I thought this was a TryFrom (Update: this is what the changelog for gix-actor said), but I guess that changed (or my IDE failed to find the TryFrom).

For consistency, one could add a TryFrom that calls to_owned() under the hood.
I prefer to_owned() if given the choice as I don't find try_from() very discoverable, but that might also be affected by the IDE.

Please feel free to submit a PR for that though.

@Byron Byron enabled auto-merge June 6, 2025 02:59
@Byron Byron merged commit 8f6ecfe into GitoxideLabs:main Jun 6, 2025
23 checks passed
ilyagr added a commit to jj-vcs/jj that referenced this pull request Jun 6, 2025
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. The signature of `gix::reference::iter::Platform::prefixed` changed
   in a way that seems to confuse Rust compiler (and does confuse me).

2. `git_object::Tree::EntryMode` API changed; `entry.mode()` now has a
   `value()` method.

3. Most significantly, the meaning and API of `gix::actor::SignatureRef`
   changed.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
github-merge-queue bot pushed a commit to jj-vcs/jj that referenced this pull request Jun 6, 2025
Update `gix` to 0.72.1, and adapt to its breaking changes.

1. The signature of `gix::reference::iter::Platform::prefixed` changed
   in a way that seems to confuse Rust compiler (and does confuse me).

2. `git_object::Tree::EntryMode` API changed; `entry.mode()` now has a
   `value()` method.

3. Most significantly, the meaning and API of `gix::actor::SignatureRef`
   changed.

## Details about `gix::actor::SignatureRef`

The API for `gix::actor::Signature` and `gix::actor::SignatureRef`
changedd. The latter now contains an unparsed string time field, while
the former still contains a parsed time.  So, the conversions between
`gix::actor::SignatureRef` and either `gix::actor::Signature` or jj's
`Signature` types can now fail.

We use the epoch for the time if the timestamp is unreadable, like gix
did before.

Cc: GitoxideLabs/gitoxide#1935,
GitoxideLabs/gitoxide#2038
@ilyagr ilyagr deleted the signature-doc branch June 8, 2025 03:41
@ilyagr
Copy link
Contributor Author

ilyagr commented Jun 8, 2025

Thank you very much for the review! Your edits are also great; the final result seems clear and helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants